HDFS-14856. Fetch file ACLs while mounting external store.#1478
HDFS-14856. Fetch file ACLs while mounting external store.#1478virajith merged 12 commits intoapache:trunkfrom
Conversation
There was a problem hiding this comment.
Should we refer to PROVIDED here?
There was a problem hiding this comment.
Agree. WDYT about dfs.namenode.provided.acls.enabled?
Keep namenode since the config is a NN config.
Replaced mount with provided so that the config name is consistent.
There was a problem hiding this comment.
Better than before for sure.
Not sure about the end policy for distinguishing namenode from not.
It should be consistent.
There was a problem hiding this comment.
Do we support adding null to the acls?
Is there a finer grain exception then IOException?
There was a problem hiding this comment.
ACL can be null.
IOException comes from the FileSystems API, AclStatus getAclStatus(Path path) throws IOException
There was a problem hiding this comment.
This one we don't care about the IOE?
There was a problem hiding this comment.
Good catch. The scanner will not fail if the remote FS does not support ACLs. All other IOException fail the operation in all paths now. Thanks !
There was a problem hiding this comment.
@ashvina - is it possible to handle the exception the same way in both the constructors?
There was a problem hiding this comment.
Why do we have this just for doing a get?
There was a problem hiding this comment.
The user mapping behavior can be different in a subclass. For e.g. In this case SingleUGIResolver maps all users to a single user.
|
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
So, this is not currently written to the image?
There was a problem hiding this comment.
Yes. I am proposing to fix the scanner in the PR which is independent of the ImageWriter. The consumer of the scanner can choose to ignore the ACL.
There was a problem hiding this comment.
Can you make this explicit (e.g,, add javadoc for this method)?
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
@ashvina - is it possible to handle the exception the same way in both the constructors?
...op-tools/hadoop-fs2img/src/main/java/org/apache/hadoop/hdfs/server/namenode/UGIResolver.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
change to javadoc style comment.
There was a problem hiding this comment.
Can we have a unit test checking for this string?
There was a problem hiding this comment.
No. Static will not work as resolution methods in the subclass may not be static.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
virajith
left a comment
There was a problem hiding this comment.
+1 once the comments are resolved and yetus issues (mostly checkstyle; failed tests seem unrelated) are resolved.
|
💔 -1 overall
This message was automatically generated. |
d2434b7 to
73140e1
Compare
|
💔 -1 overall
This message was automatically generated. |
...tools/hadoop-fs2img/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSTreeWalk.java
Outdated
Show resolved
Hide resolved
...tools/hadoop-fs2img/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSTreeWalk.java
Outdated
Show resolved
Hide resolved
...tools/hadoop-fs2img/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSTreeWalk.java
Outdated
Show resolved
Hide resolved
...tools/hadoop-fs2img/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSTreeWalk.java
Outdated
Show resolved
Hide resolved
d2c8ba4 to
1bd5ee0
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Test failures are unrelated. Completing this PR. |
Addresses https://issues.apache.org/jira/browse/HDFS-14856.
If configuration
dfs.namenode.mount.acls.enabledis set (true),FsTreeWalkwill fetch ACLs of files on the external storage system provided at the time of mount.